Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix for writeProperty fail #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix for writeProperty fail #117

wants to merge 1 commit into from

Conversation

bing-mao
Copy link

@bing-mao bing-mao commented Aug 29, 2018 •
edited
Loading

Checklist

  • I've read and followed the Contribution Guide.
  • My commit messages reference affected issues and mention breaking changes if applicable.
  • I've updated / wrote inline documentation for the source code affected by my changes.
  • All mandatory CI checks have passed (see when Pull Request has been submitted).

Open Question

What does this Pull Request do

@fh1ch fh1ch self-assigned this Mar 7, 2019
@DiniFarb
Copy link

DiniFarb commented Feb 18, 2022 •
edited
Loading

I was facing the same issue as described in #138.

I digged in and found that in the writeProperty function, if no priority in the optional options object is set the value will be undefined and then not set in the protocol.
Speaking of this function:

node-bacstack/lib/client.js

Line 615 in 9e19310

writeProperty(address, objectId, propertyId, values, options, next) {

Here a test with priority in options set (left side) and without (right side).
tempsnip

So I was about to create a PR to fix it but luckely I came across this PR and saw that it already has 😄

Therefore just wanted to confirm that this PR solves #138 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@fh1ch fh1ch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bing-mao @DiniFarb @fh1ch